subsys: add MODBUS subsystem #26516

jfischer-no · 2020-06-29T11:22:17Z

Add MODBUS ~~RTU (over serial line)~~ subsystem.
MODBUS ~~RTU~~ implementation supports booth server and
client roles. Some components of the implementation are based
on the uC/Modbus stack, which was published under Apache license.

alexanderwachter

A brief review of the "main" commit.

subsys/modbus/mb_rtu_core.c

subsys/modbus/mb_rtu_client.c

subsys/modbus/mb_rtu_internal.h

subsys/modbus/mb_rtu_server.c

carlescufi · 2020-07-10T11:31:03Z

subsys/modbus/mb_rtu_internal.h

+	atomic_t state;
+
+#ifdef CONFIG_MODBUS_RTU_FC08_DIAGNOSTIC
+	uint16_t mbs_msg_ctr;


do you need the mbs_ prefix in those? they are already namespaced inside the struct

The only reason is that this counters are used only by the server (MODBUS_RTU_FC08_DIAGNOSTIC depends on server mode).

carlescufi

Should this live under subsys/mgmt/modbus?

Add function to get Modbus RTU interface index according to interface name. This can be used to clearly identify interfaces in the application. Signed-off-by: Johann Fischer <[email protected]>

Fix ASCII frame reception and add test for ASCII mode. Signed-off-by: Johann Fischer <[email protected]>

Reset buffer after reception, even if an error occurs. Signed-off-by: Johann Fischer <[email protected]>

Rename MODBUS RTU API to common MODBUS API. Signed-off-by: Johann Fischer <[email protected]>

Remove RTU from configuration and headers Signed-off-by: Johann Fischer <[email protected]>

Rename mb_rtu_ to modbus_. Signed-off-by: Johann Fischer <[email protected]>

Remove _rtu_ from modbus test. Signed-off-by: Johann Fischer <[email protected]>

Use enum for MODBUS mode. Signed-off-by: Johann Fischer <[email protected]>

Move MODBUS over serial line code to own source file. Signed-off-by: Johann Fischer <[email protected]>

Move serial line config outside context. Signed-off-by: Johann Fischer <[email protected]>

Make MODBUS support over serial line optional. Signed-off-by: Johann Fischer <[email protected]>

Prefix internal functions and structs with modbus_. Use unit_id consistently instead of node_addr. Fix mbm_ remainder and rename to mbc_. Rename struct modbus_frame to modbus_adu since ADU is closer to what the structure represents. Let the compiler/linker do the job and remove ifdef around mbc_validate_fc03fp_response(). Signed-off-by: Johann Fischer <[email protected]>

Let the core call the modbus_tx_adu() to make the process more comprehensible. Move tx-wait-for-rx handling outside of client code. Signed-off-by: Johann Fischer <[email protected]>

Return ETIMEDOUT on timeout instead of EIO. Signed-off-by: Johann Fischer <[email protected]>

Document return values of internal functions. Signed-off-by: Johann Fischer <[email protected]>

Use commot parameter structure to configure server or client interfaces. Signed-off-by: Johann Fischer <[email protected]>

MODBUS raw ADU support allows to implement MODBUS messaging service over TCP or UDP. Signed-off-by: Johann Fischer <[email protected]>

Add raw ADU support test. Signed-off-by: Johann Fischer <[email protected]>

Add MODBUS TCP server sample. Signed-off-by: Johann Fischer <[email protected]>

Add TCP to serial line gateway. Signed-off-by: Johann Fischer <[email protected]>

Update description and add TCP sample references. Signed-off-by: Johann Fischer <[email protected]>

Follow modern way to configure LEDs GPIO using struct gpio_dt_spec and GPIO_DT_SPEC_GET. Signed-off-by: Johann Fischer <[email protected]>

Follow modern way to configure DE,RE GPIO using struct gpio_dt_spec. Signed-off-by: Johann Fischer <[email protected]>

carlescufi

The scancode toolkit failures are false positives. The files include an SPDX identifier for Apache v2.

anangl

I'm a bit late with my comments, but I'm adding them anyway, just in case there will be a follow up PR.

anangl · 2021-03-19T14:10:28Z

samples/subsys/modbus/rtu_server/src/main.c

+
+static int init_modbus_server(void)
+{
+	const uint32_t mb_rtu_br = 19200;


This does not seem to be used.

anangl · 2021-03-19T14:18:08Z

samples/subsys/modbus/rtu_server/src/main.c

+static int init_modbus_server(void)
+{
+	const uint32_t mb_rtu_br = 19200;
+	const char iface_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};


This way you create a local array (on stack) and fill it with the provided string (those brackets are actually confusing). You need to use then log_strdup(iface_name) below, otherwise its content will be lost.
But I guess this was not intended and the following would work here:

const char *iface_name = DT_PROP(DT_INST(0, zephyr_modbus_serial), label);

@anangl agree, Agree. Nicer to see the latter.

The const will make sure that this data is stored in RO region (flash). How is stack related here?

anangl · 2021-03-19T14:34:21Z

samples/subsys/modbus/rtu_client/src/main.c

+
+static int init_modbus_client(void)
+{
+	const char iface_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};


See the corresponding comment in samples/subsys/modbus/rtu_server/src/main.c.

Suggested change

const char iface_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};

const char *iface_name = DT_PROP(DT_INST(0, zephyr_modbus_serial), label);

anangl · 2021-03-19T14:36:39Z

samples/subsys/modbus/rtu_client/src/main.c

+		err = modbus_write_coil(client_iface, node, addr++, true);
+		if (err != 0) {
+			LOG_ERR("FC05 failed with %d", err);
+			return;
+		}
+
+		k_msleep(sleep);


This could be placed in a loop, to avoid code duplication.

anangl · 2021-03-19T14:47:00Z

dts/bindings/misc/zephyr,modbus-serial.yaml

+        If connected directly the MCU pin should be configured
+        as active high.


I think this might be confusing, as in both provided overlays this signal is actually active low. Perhaps this sentence could be dropped. Similarly in the RE case. For the same reason, "nRE" and "DE" markings in descriptions could be dropped as well. I don't think they actually bring much value.

anangl · 2021-03-19T14:54:50Z

samples/subsys/modbus/tcp_gateway/src/main.c

+
+static int init_backend_iface(void)
+{
+	const char bend_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};


See the corresponding comment in samples/subsys/modbus/rtu_server/src/main.c.

Suggested change

const char bend_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};

const char *bend_name = DT_PROP(DT_INST(0, zephyr_modbus_serial), label);

After this change, you won't need this log_strdup below.

jfischer-no added this to the v2.4.0 milestone Jun 29, 2020

jfischer-no requested review from nashif, henrikbrixandersen, alexanderwachter and carlescufi June 29, 2020 11:22

zephyrbot added area: Devicetree area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test area: Documentation labels Jun 29, 2020

jfischer-no mentioned this pull request Jun 29, 2020

drivers: uart_mcux: add UART set configuration function #26517

Merged

alexanderwachter requested changes Jun 29, 2020

View reviewed changes

github-actions bot added area: Device Tree area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jun 30, 2020

galak removed the area: Device Tree label Jul 1, 2020

github-actions bot added the platform: NXP NXP label Jul 7, 2020

carlescufi reviewed Jul 10, 2020

View reviewed changes

carlescufi mentioned this pull request Jul 28, 2020

Add support for Open Supervised Device Protocol (OSDP) #26965

Merged

This comment has been minimized.

Sign in to view

parthitce mentioned this pull request Aug 8, 2020

Porting Modbus Library: Need some guidance #10022

Closed

jfischer-no force-pushed the pr-modbus-rtu branch from 0af6249 to 07e8d80 Compare August 21, 2020 07:42

github-actions bot added the area: Samples Samples label Aug 21, 2020

jfischer-no force-pushed the pr-modbus-rtu branch 2 times, most recently from 9f81f07 to c93f5e6 Compare August 31, 2020 12:53

jfischer-no marked this pull request as ready for review August 31, 2020 12:54

jfischer-no requested review from galak and MaureenHelm as code owners August 31, 2020 12:54

carlescufi reviewed Aug 31, 2020

View reviewed changes

martinjaeger self-requested a review September 1, 2020 16:38

jfischer-no added 23 commits March 19, 2021 13:56

modbus: get interface index according to interface name

ec59d02

Add function to get Modbus RTU interface index according to interface name. This can be used to clearly identify interfaces in the application. Signed-off-by: Johann Fischer <[email protected]>

modbus: fix ASCII frame reception and add test for ASCII mode

256ff7e

Fix ASCII frame reception and add test for ASCII mode. Signed-off-by: Johann Fischer <[email protected]>

modbus: reset buffer after reception

e941c2c

Reset buffer after reception, even if an error occurs. Signed-off-by: Johann Fischer <[email protected]>

modbus: rename MODBUS RTU API to common MODBUS API

8becbb3

Rename MODBUS RTU API to common MODBUS API. Signed-off-by: Johann Fischer <[email protected]>

modbus: remove RTU from configuration and headers

86879c9

Remove RTU from configuration and headers Signed-off-by: Johann Fischer <[email protected]>

modbus: rename mb_rtu_ to modbus_

38f8315

Rename mb_rtu_ to modbus_. Signed-off-by: Johann Fischer <[email protected]>

tests: modbus: remove _rtu_ from modbus test

51151f0

Remove _rtu_ from modbus test. Signed-off-by: Johann Fischer <[email protected]>

modbus: use enum for MODBUS mode

5c015f0

Use enum for MODBUS mode. Signed-off-by: Johann Fischer <[email protected]>

modbus: move MODBUS over serial line code to own file

87f748b

Move MODBUS over serial line code to own source file. Signed-off-by: Johann Fischer <[email protected]>

modbus: move serial line config outside context

d2eaf59

Move serial line config outside context. Signed-off-by: Johann Fischer <[email protected]>

modbus: make MODBUS support over serial line optional

d157027

Make MODBUS support over serial line optional. Signed-off-by: Johann Fischer <[email protected]>

modbus: move some RX/TX ADU related code to the core

766d36f

Let the core call the modbus_tx_adu() to make the process more comprehensible. Move tx-wait-for-rx handling outside of client code. Signed-off-by: Johann Fischer <[email protected]>

modbus: return ETIMEDOUT on timeout instead of EIO

7752b88

Return ETIMEDOUT on timeout instead of EIO. Signed-off-by: Johann Fischer <[email protected]>

modbus: document return values of internal functions

87d50a4

Document return values of internal functions. Signed-off-by: Johann Fischer <[email protected]>

modbus: rework interface configuration

828e468

Use commot parameter structure to configure server or client interfaces. Signed-off-by: Johann Fischer <[email protected]>

modbus: add raw ADU support

2290543

MODBUS raw ADU support allows to implement MODBUS messaging service over TCP or UDP. Signed-off-by: Johann Fischer <[email protected]>

tests: modbus: add raw ADU support test

bd6f51f

Add raw ADU support test. Signed-off-by: Johann Fischer <[email protected]>

samples: add MODBUS TCP server sample

dc60733

Add MODBUS TCP server sample. Signed-off-by: Johann Fischer <[email protected]>

samples: modbus: add TCP to serial line gateway

6424f1e

Add TCP to serial line gateway. Signed-off-by: Johann Fischer <[email protected]>

doc: modbus: update description and add TCP sample references

876a064

Update description and add TCP sample references. Signed-off-by: Johann Fischer <[email protected]>

samples: rtu_server: rework LED configuration

cb066a8

Follow modern way to configure LEDs GPIO using struct gpio_dt_spec and GPIO_DT_SPEC_GET. Signed-off-by: Johann Fischer <[email protected]>

modbus: rework RS-485 transceiver DE,RE GPIO configuration

cda8c30

Follow modern way to configure DE,RE GPIO using struct gpio_dt_spec. Signed-off-by: Johann Fischer <[email protected]>

jfischer-no force-pushed the pr-modbus-rtu branch from 91400b3 to cda8c30 Compare March 19, 2021 12:57

carlescufi approved these changes Mar 19, 2021

View reviewed changes

carlescufi merged commit f943a55 into zephyrproject-rtos:master Mar 19, 2021

anangl reviewed Mar 19, 2021

View reviewed changes

jfischer-no deleted the pr-modbus-rtu branch September 15, 2021 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

subsys: add MODBUS subsystem #26516

subsys: add MODBUS subsystem #26516

jfischer-no commented Jun 29, 2020 •

edited

Loading

alexanderwachter left a comment

carlescufi Jul 10, 2020

jfischer-no Jul 10, 2020 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

carlescufi left a comment

carlescufi left a comment

anangl left a comment

anangl Mar 19, 2021

anangl Mar 19, 2021

manoj153 Mar 22, 2021

anangl Mar 19, 2021

anangl Mar 19, 2021

anangl Mar 19, 2021

anangl Mar 19, 2021

	const char iface_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};
	const char *iface_name = DT_PROP(DT_INST(0, zephyr_modbus_serial), label);

		If connected directly the MCU pin should be configured
		as active high.

	const char bend_name[] = {DT_PROP(DT_INST(0, zephyr_modbus_serial), label)};
	const char *bend_name = DT_PROP(DT_INST(0, zephyr_modbus_serial), label);

subsys: add MODBUS subsystem #26516

subsys: add MODBUS subsystem #26516

Conversation

jfischer-no commented Jun 29, 2020 • edited Loading

alexanderwachter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfischer-no Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

carlescufi left a comment

Choose a reason for hiding this comment

carlescufi left a comment

Choose a reason for hiding this comment

anangl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfischer-no commented Jun 29, 2020 •

edited

Loading

jfischer-no Jul 10, 2020 •

edited

Loading